Fix for addons using ember-cli-typescript 4x #529
Conversation
packages/compat/src/v1-addon.ts
Outdated
| @@ -429,6 +444,10 @@ export default class V1Addon { | |||
| if (this.templateCompiler) { | |||
| babelConfig.plugins.push(this.templateCompiler.inlineTransformsBabelPlugin()); | |||
| } | |||
|
|
|||
| if (this.needsTypeScriptTransform) { | |||
| babelConfig.plugins.push(require.resolve('@babel/plugin-transform-typescript')); | |||
There was a problem hiding this comment.
I didn't think this would be necessary, because ember-cli-babel adds the typescript plugin when it sees that ember-cli-typescript 4x is present?
Your fix above in needsCustomBabel should ensure that we allow ember-cli-babel to do its usual thing (other than the changes we make to its own settings like compileModules: false).
If this line is really needed to make it work, we should debug why because then I don't understand it.
There was a problem hiding this comment.
Also, if we do keep this line, we should probably guard against double insertion of the plugin. If anybody else has already inserted it, babel will throw when it sees the duplicate.
For an example of where we already do this, see
embroider/packages/webpack/src/ember-webpack.ts
Lines 500 to 502 in 3d48872
There was a problem hiding this comment.
Also, this is resolving the plugin relative to @embroider/compat, which doesn't have a dependency on @babel/plugin-transform-typescript. It needs to resolve relative to ember-cli-babel's root, because AFAIK that is the package that depends on the plugin. But again, this is one more reason I think it's suspicious to add the plugin here -- I think ember-cli-babel should probably be doing it.
There was a problem hiding this comment.
You're totally right. All it needed was a6567f2!
I'm not sure how I ended up down the rabbit hole where I thought I needed to have Embroider add the plugin. ¯\_(ツ)_/¯
jamescdavis
force-pushed
the
e-c-ts-4-addon-fix
branch
2 times, most recently
from
701009a to
4bdec90
Compare
jamescdavis
force-pushed
the
e-c-ts-4-addon-fix
branch
from
4bdec90 to
a6567f2
Compare
|
This is marked draft still but I think it's ready? |
|
@ef4 I've been trying to add an app that tests this |
|
I'm hesitant to keep adding separate test apps anyway. It's a lot to maintain and it's slow. People keep doing it because it's more obvious, but I'd encourage more tests that look like these: https://github.com/embroider-build/embroider/blob/master/packages/compat/tests/app-import.test.ts |
|
Yeah, it's very slow. Those tests are a much better strategy. |
Fixes #528
We need to trigger a custom Babel run for any addons using ember-cli-typescript 4.